-
Notifications
You must be signed in to change notification settings - Fork 92
Update more places with respect to ref-valued properties and indexers #1402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: draft-v8
Are you sure you want to change the base?
Conversation
standard/expressions.md
Outdated
| A member lookup is the process whereby the meaning of a name in the context of a type is determined. A member lookup can occur as part of evaluating a *simple_name* ([§12.8.4](expressions.md#1284-simple-names)) or a *member_access* ([§12.8.7](expressions.md#1287-member-access)) in an expression. If the *simple_name* or *member_access* occurs as the *primary_expression* of an *invocation_expression* ([§12.8.10.2](expressions.md#128102-method-invocations)), the member is said to be *invoked*. | ||
| If a member is a method or event, or if it is a constant, field or property of either a delegate type ([§20](delegates.md#20-delegates)) or the type `dynamic` ([§8.2.4](types.md#824-the-dynamic-type)), then the member is said to be *invocable.* | ||
| If a member is a method or event, or if it is a constant, field or property whose evaluated value is either of a delegate type ([§20](delegates.md#20-delegates)) or the type `dynamic` ([§8.2.4](types.md#824-the-dynamic-type)), then the member is said to be *invocable.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change enables:
class C
{
void M()
{
Prop();
}
private System.Action f;
public ref System.Action Prop => ref f;
}Since the property type is ref System.Action which is not a delegate type, but the property's evaluated value is of type System.Action.
standard/expressions.md
Outdated
| A member initializer that specifies an expression after the equals sign is processed in the same way as an assignment ([§12.21.2](expressions.md#12212-simple-assignment)) to the target. | ||
|
|
||
| A member initializer that specifies an object initializer after the equals sign is a ***nested object initializer***, i.e., an initialization of an embedded object. Instead of assigning a new value to the field or property, the assignments in the nested object initializer are treated as assignments to members of the field or property. Nested object initializers cannot be applied to properties with a value type, or to read-only fields with a value type. | ||
| A member initializer that specifies an object initializer after the equals sign is a ***nested object initializer***, i.e., an initialization of an embedded object. Instead of assigning a new value to the field or property, the assignments in the nested object initializer are treated as assignments to members of the field or property. Nested object initializers cannot be applied to properties or indexers with a value type, to ref-valued properties or indexers whose type is a reference to a value type, or to read-only fields with a value type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likely fixes an older spec bug which failed to mention that this rule applies to indexers as well as properties.
| A member initializer that specifies an object initializer after the equals sign is a ***nested object initializer***, i.e., an initialization of an embedded object. Instead of assigning a new value to the field or property, the assignments in the nested object initializer are treated as assignments to members of the field or property. Nested object initializers cannot be applied to properties or indexers with a value type, to ref-valued properties or indexers whose type is a reference to a value type, or to read-only fields with a value type. | ||
|
|
||
| A member initializer that specifies a collection initializer after the equals sign is an initialization of an embedded collection. Instead of assigning a new collection to the target field, property, or indexer, the elements given in the initializer are added to the collection referenced by the target. The target shall be of a collection type that satisfies the requirements specified in [§12.8.17.2.3](expressions.md#1281723-collection-initializers). | ||
| A member initializer that specifies a collection initializer after the equals sign is an initialization of an embedded collection. Instead of assigning a new collection to the target field, property, or indexer, the elements given in the initializer are added to the collection referenced by the target. The target shall be of a collection type that satisfies the requirements specified in [§12.8.17.2.3](expressions.md#1281723-collection-initializers). Initialization of an embedded collection cannot be applied to properties or indexers with a value type, to ref-valued properties or indexers whose type is a reference to a value type, or to read-only fields with a value type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likely fixes an older spec bug which failed to mention that this rule applies to nested collection initializers as well as nested object initializers.
standard/classes.md
Outdated
| - A property that has only a get accessor is said to be a ***read-only property***. It is a compile-time error for a read-only property to be the target of an assignment. | ||
| - A property that has only a set accessor is said to be a ***write-only property***. Except as the target of an assignment, it is a compile-time error to reference a write-only property in an expression. | ||
| - A property that has only a get accessor is said to be a ***read-only property***. It is a compile-time error for a read-only property to be the target of an assignment unless the property is ref-valued and returns a writeable reference. | ||
| - A property that has only a set accessor is said to be a ***write-only property***. Except as the target of an assignment or as an argument to the `nameof` operator ([§12.8.23](expressions.md#12823-the-nameof-operator)), it is a compile-time error to reference a write-only property in an expression. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likely fixes an older spec bug which was disallowing:
class C
{
void M()
{
_ = nameof(Prop);
}
public int Prop { set { } }
}|
@jnm2 Would you like us to review this and discuss it in the upcoming meeting after the priority PRs? If so, feel free to apply the meeting-discuss label. |
|
Sure. Is applying that label a good automatic step to take when submitting a PR that is ready for review? |
If it's non-trivial, yes. For very small changes, it's fine to just get appropriate folks to review between meetings and merge. |
standard/expressions.md
Outdated
| - The best indexer of the set of candidate indexers is identified using the overload resolution rules of [§12.6.4](expressions.md#1264-overload-resolution). If a single best indexer cannot be identified, the indexer access is ambiguous, and a binding-time error occurs. | ||
| - The accessors of the best indexer are checked: | ||
| - If the indexer access is the target of an assignment then the indexer shall have a set or ref get accessor, otherwise, a binding-time error occurs; | ||
| - If the indexer access is the target of an assignment then the indexer either shall have a set accessor or shall return a non-readonly ref, otherwise, a binding-time error occurs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes a spec issue where it would be valid to assign to a ref readonly indexer. (That is, an indexer whose returned ref is a readonly ref. I'm not talking about a readonly ref indexer, which is an indexer which is a readonly struct member.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably in classes on indexers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm not sure what you're saying. Is there an open question on the change here or another place to update?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry. I was saying the update for ref readonly should be in the section on indexers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read https://github.com/dotnet/csharpstandard/blob/draft-v8/standard/classes.md#1591-general but I don't see anything describing assignments to indexers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, that was the wrong link. FIxed!
BillWagner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good progress. I'd also like to see more xrefs to15.7.1 where "non-ref-valued" and "ref-valued" properties are defined.
standard/expressions.md
Outdated
| - The best indexer of the set of candidate indexers is identified using the overload resolution rules of [§12.6.4](expressions.md#1264-overload-resolution). If a single best indexer cannot be identified, the indexer access is ambiguous, and a binding-time error occurs. | ||
| - The accessors of the best indexer are checked: | ||
| - If the indexer access is the target of an assignment then the indexer shall have a set or ref get accessor, otherwise, a binding-time error occurs; | ||
| - If the indexer access is the target of an assignment then the indexer either shall have a set accessor or shall return a non-readonly ref, otherwise, a binding-time error occurs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably in classes on indexers.
|
@BillWagner Thanks! The xrefs have been added. |
jskeet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm generally very much in favour of this - just a few nits.
| - The value of a variable is simply the value currently stored in the storage location identified by the variable. A variable shall be considered definitely assigned ([§9.4](variables.md#94-definite-assignment)) before its value can be obtained, or otherwise a compile-time error occurs. | ||
| - The value of a property access expression is obtained by invoking the get accessor of the property. If the property has no get accessor, a compile-time error occurs. Otherwise, a function member invocation ([§12.6.6](expressions.md#1266-function-member-invocation)) is performed, and the result of the invocation becomes the value of the property access expression. | ||
| - The value of an indexer access expression is obtained by invoking the get accessor of the indexer. If the indexer has no get accessor, a compile-time error occurs. Otherwise, a function member invocation ([§12.6.6](expressions.md#1266-function-member-invocation)) is performed with the argument list associated with the indexer access expression, and the result of the invocation becomes the value of the indexer access expression. | ||
| - The value of a property access expression is obtained by invoking the get accessor of the property. If the property has no get accessor, a compile-time error occurs. Otherwise, a function member invocation ([§12.6.6](expressions.md#1266-function-member-invocation)) is performed. For non-ref-valued properties (§15.7.1), the result of the invocation becomes the value of the property access expression. For ref-valued properties, the result of the invocation is a variable and the value of the variable becomes the value of the property access expression. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about "and the value of the variable becomes the value of the property access expression". We've classified it as a variable, not a value... Can we not just leave it as something like "and that variable is the result of evaluating the get accessor"? I may very well be wrong on this, and haven't checked other places yet.
Same applies to the line below, of course.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nigel: "the variable reference becomes the result of the property access expression"
Or maybe "the variable becomes" - please check for use of "variable reference" elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See also #324 and clause 9.5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec currently uses variable reference 33 times. For instance, https://github.com/dotnet/csharpstandard/blob/draft-v8/standard/arrays.md#174-array-element-access:
The result of an array access is a variable reference (§9.5) to the array element selected by the indices.
"Variable references" (https://github.com/dotnet/csharpstandard/blob/draft-v8/standard/variables.md#95-variable-references) defines "variable reference" as the expression that refers to the variable, rather than the value of, or the result of evaluating, the expression. However, the array access spec uses "variable reference" to mean the result, so I think I'll maintain consistency with the array access spec. If #324 comes to some additional clarity that would change the array access spec, then it will also pick up this section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to back up a second. I'm not sure we can say that the value is a variable or variable reference. This section starts with this bullet point:
- The value of a variable is simply the value currently stored in the storage location identified by the variable. A variable shall be considered definitely assigned (§9.4) before its value can be obtained, or otherwise a compile-time error occurs.
That bullet point doesn't say "the value of a variable is a variable" or "variable reference." It says the value is the contents of the variable, not the variable itself. So if the value of the expression myVariable is not a variable, why would the value of the expression obj[p] be a variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I took this section to be the section which explains how a usage of myVariable ends up being a read of the contents of the variable. So e.g. if you see M(myVariable);, this section describes how it is that we're obtaining the value of that variable rather than e.g. passing a variable itself. Wouldn't the same hold for M(obj[p]);?)
Co-authored-by: Jon Skeet <[email protected]>
5073ab7 to
d992f2a
Compare
Fixes #1371
I spread a wide net and put no effort toward deduplicating so far, after seeing the wide variety in types of update. I am open to suggestion on that.